Control Screensaver Animation Speed Acroos Different Hardware & Emulator#733
Control Screensaver Animation Speed Acroos Different Hardware & Emulator#7331abdelhalim wants to merge 2 commits intoSeedSigner:devfrom
Conversation
|
If I'm reading this right, your approach sets an effective max frame rate (faster screen refresh cycles are ignored until the threshold time has elapsed). But the velocity / pixels-per-update is not altered based on any frame rendering speed metric. So just making up numbers, for a given initial rand velocity, we might see:
The more powerful setups might be capable of, say, 60fps but this approach will restrict it to the max effective frame rate. INSTEAD: I was thinking that the rand velocity would be constant on a per-second basis (e.g. 30 total pixels moved per second no matter what hardware it runs on) but it would be chopped up into This way we gain:
|
|
You're right, your approach is better. I was mainly focusing on controlling the speed, but I now see how your idea will provide smoother motion across all devices. |
|
It should now behave consistently across all hardware. Appreciate any feedback |
src/seedsigner/views/screensaver.py
Outdated
| # pixels to move per second | ||
| self.pixels_per_second = 150.0 | ||
|
|
||
| # Direction of movement in x and y |
There was a problem hiding this comment.
If I'm reading this right, these vars aren't just the direction but rather a kind of magnitude scalar for each axis.
In which case I don't think "direction" is the right naming convention to use. At the moment "scalar" is the best term I can think of. Or "direction_multiplier". Or "delta_rate_x"? "cur_vector" with x/y component?
| font = Fonts.get_font(GUIConstants.get_body_font_name(), GUIConstants.get_top_nav_title_font_size()) | ||
| version = f"v{controller.VERSION}" | ||
|
|
||
| # The logo png is 240x240, but the actual logo is 70px tall, vertically centered |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
|
||
| background = Image.new("RGBA", size=self.logo.size, color="black") | ||
| if not self.is_screenshot_renderer: | ||
| # Fade in alpha |
There was a problem hiding this comment.
We always try to over-explain things via comments. Do not remove unless a comment is wrong, no longer relevant, etc.
- Simplified `rand_speed()` to return values between `min_speed` and `max_speed`. - Made speed scale with screen’s smallest dimension. - Renamed direction vars to `vector_x` / `vector_y`. - Casted `cur_x` / `cur_y` to integers to fix rendering issues.
bc4769a to
d1ab2e4
Compare
| min_screen_dimension = min(self.renderer.canvas_width, self.renderer.canvas_height) | ||
| # Min speed: cross the screen in 3 seconds, max: cross in 1 second | ||
| self.min_speed = min_screen_dimension / 3.0 # pixels per second | ||
| self.max_speed = min_screen_dimension / 1.0 # pixels per second |
There was a problem hiding this comment.
In my testing, min / max speeds worked best if it took 1.5s - 10s to cross the screen in any one dimension.
But note that because of your rand_direction implementation, the speeds specified here do NOT correspond to the actual min/max speed you're achieving.
min_speed is X but you're scaling it by a fractional rand_direction so the actual minimum speed can be much slower, approaching zero.
|
The current iteration of this strikes me as a confused in-between version that does not fully incorporate updated ideas while still being burdened by some old ones. As noted in my comment above, min/max speed are not the actual min/max achieved. The I hacked around in your code and simplified it to:
|
| self.increment_y *= -1.0 | ||
| # Handle boundary collisions and ensure we don't get stuck | ||
| if self.cur_x <= self.min_coords[0]: | ||
| self.cur_x = self.min_coords[0] + 1 # Move slightly away from edge |
There was a problem hiding this comment.
No need to worry about 1 pixel differences. The logo bounce is calculated from the center of the logo; there aren't any extreme edge scenarios to worry about.
|
We have an easy fix for this bug related to the screensaver. Please refer to the following link for the proposed solution: #811. |
Original PR seems too complicated but this new PR almost seems not complicated enough 🤓 |
Thanks for your feedback! I actually tested the solution in |
|
@1abdelhalim will you update this PR based on the latest review comments? |
Description
This PR ensures the screensaver animation runs at a consistent speed across all Raspberry Pi models, including the SeedSigner Emulator.
Previously, the logo moved too fast on more powerful devices due to differences in hardware rendering speeds.
Changes:
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before submitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
No, I’m a fool
Yes
N/A
I have tested this PR on:
SeedSigner Emulator